-
Notifications
You must be signed in to change notification settings - Fork 4.8k
HIVE-29403: Move the truncate table out of HMSHandler #6269
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
b5a0a89 to
bf64176
Compare
bf64176 to
e305e22
Compare
|
cc @wecharyu @saihemanth-cloudera could you take a look if have secs? thank you! |
e305e22 to
0daab9d
Compare
...tastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/JavaUtils.java
Outdated
Show resolved
Hide resolved
...re-server/src/main/java/org/apache/hadoop/hive/metastore/handler/AbstractRequestHandler.java
Outdated
Show resolved
Hide resolved
| Class<? extends TBase> requestBody(); | ||
| boolean supportAsync() default false; | ||
| String id() default "getId"; | ||
| String cancel() default "isCancel"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
id and cancel is from request, it seems a little strange to set get such annotations for handler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when creating the handler, it knows what the request is, and how to get the async id from the request at the definition.
| private Future<Result> executeRequest() { | ||
| Timer.Context timerContext; | ||
| if (StringUtils.isNotEmpty(getMetricAlias())) { | ||
| Timer timer = Metrics.getOrCreateTimer(MetricsConstants.API_PREFIX + getMetricAlias()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will multi reflection calls affect the performance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's just a simple call, no reflection here
| } finally { | ||
| super.afterExecute(result); | ||
| if (async) { | ||
| rs.shutdown(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why cleanup rs here? it will be cleaned if connection is closed in HMSHandler.cleanupHandlerContext().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, if this handler is running in background thread, HMSHandler.cleanupHandlerContext() will just clean the main thread which creates this backed thread.
| } finally { | ||
| super.afterExecute(result); | ||
| if (async) { | ||
| ms.shutdown(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto.
|



What changes were proposed in this pull request?
Why are the changes needed?
Does this PR introduce any user-facing change?
How was this patch tested?